Repository navigation
Conversation
…g with EAGAIN The copy loops behind Bun.write(file, file) treated EAGAIN as a failure. A pipe or socket on fd 1 is non-blocking once process.stdout is used, so `await Bun.write(Bun.stdout, Bun.file(path))` rejected after a part of the file. On EAGAIN the copy now waits until the fd is ready and repeats the same syscall: - bun_sys::wait_until_readable / wait_until_writable block in poll, and in select on macOS, where poll does not wake when the other end of a named pipe closes. write_all_to_stdio uses them too. - do_copy_file_range (sendfile, splice) asks poll which end is not ready, then waits for it. A destination that reports an error gets one retry, for the syscall to report it. After that only the source ends the wait, because splice does not look at the destination while the source is empty. - read_write_loop_capped waits for the fd whose read or write failed.
|
Updated 1:31 PM PT - Oct 25th, 2026
✅ @robobun, your commit 320ef461f3a42077ee47d85d1d5e170f5f796362 passed in 🧪 To try this PR locally: bunx bun-pr 46858That installs a local version of the PR into your bun-46858 --bun |
|
Status
|
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Unix readiness waits src/sys/lib.rs |
Adds public readability and writability wait helpers. Non-macOS Unix uses poll; macOS uses non-cancellable select. write_all_to_stdio uses the writable wait after EAGAIN. |
Retrying file-copy operations src/sys/lib.rs, src/runtime/node/node_fs.rs, src/runtime/webcore/blob/copy_file.rs, test/js/bun/io/bun-write.test.js |
Adds retrying read and write helpers and uses them in NodeFS and blob copy paths. Linux and Android kernel-copy paths fall back after EAGAIN from sendfile or splice, but not copy_file_range. Tests check copy results and errors across non-blocking pipes, sockets, and FIFOs. |
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
Minimal · up to 320ef
-
Merge Risk: ⚪ Minimal · up to
320efThe reviewed copy paths have no identified issue requiring a fix before merge.
Pre-merge checks: 
4
-
Full details: Description check
-
Explanation
The description follows the template with both required sections, "What does this PR do?" (Problem, Fix, Background, Downsides) and "How did you verify your code works?" (test file, 7 new tests, failing on main, plus existing suites run). It is detailed and specific, and it names the affected files and functions. The main gap is that it does not verify on macOS or FreeBSD, and it says so. The bot evidence note flags that the platform-specific tests defer to CI. The PR objectives mention nine new tests, while the description says 7, so the count differs and should be reconciled. Verification is therefore substantive but partly unverified for macOS and FreeBSD.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
src/runtime/webcore/blob/copy_file.rs— pre-existing: a copy from a non-blocking pipe or socket into a path destination still rejects with EAGAIN after this change, while the fd-destination twin now waits. The Bun-opened-path branch at copy_file.rs:1011 (and copy_file.rs:556, copy_file.rs:913 on macOS/FreeBSD) hands the copy to NodeFS::copy_file_using_read_write_loop, whose read at node_fs.rs:4883 returns every error unchanged. Fix: give that loop the same EAGAIN handling as read_write_loop_capped (wait_if_would_block with wait_until_readable/wait_until_writable on read and write), or route the non-regular-source case through read_write_loop_capped, so every CopyFile destination kind waits. [also at: src/runtime/webcore/blob/copy_file.rs:556 - pre-existing: a copy from a non-blocking fd into a path destination still rejects with EAGAIN after this change, same as on the base branch.]Why this was flagged
Trigger:
Bun.write("out.bin", Bun.file(fd))where fd is a FIFO opened with O_NONBLOCK or a non-blocking socket on stdin, with a path destination. On Linux a socket source takes the Sendfile branch at copy_file.rs:850, sendfile fails EINVAL with total_written == 0 and falls to fallback_read_write at copy_file.rs:464, which calls NodeFS::copy_file_using_read_write_loop at copy_file.rs:1011. On macOS a FIFO/socket source with a path destination goes through do_fcopy_file_with_read_write_loop_fallback and reaches the same NodeFS loop at copy_file.rs:556; FreeBSD at copy_file.rs:913. That loop's Syscall::read at node_fs.rs:4883 and its write loop return any Err, including EAGAIN, so the promise rejects with EAGAIN exactly as on the base branch. The new wait_if_would_block helper (copy_file.rs:1029) is only wired into read_write_loop_capped, the fd-destination path, so the safeguard this PR adds does not reach the path-destination twin.Verification: pre-existing — the base rejects by the same route. The PR fixes
read_write_loop_capped(copy_file.rs:1055-1074) but leaves its twin untouched. Every Path destination routes throughNodeFS::copy_file_using_read_write_loop(copy_file.rs:1011, 556, 913), which returns read errors unchanged at node_fs.rs:4883-4891, so EAGAIN from a non-blocking source rejects the promise while the fd-destination twin now waits.
…AIN from sendfile and splice to them `bun_sys::read_retrying` and `write_retrying` wait for a non-blocking fd that is not ready, as a blocking fd does. `read_write_loop_capped` and the node:fs loop call them. `do_copy_file_range` no longer waits and repeats `sendfile` or `splice`. On EAGAIN it continues the copy in the read/write loop. This removes `wait_for_kernel_copy` and its rule for the end to wait for. The tests cover the continuation after `sendfile` and `splice` copied a part, both waits with a CPU limit, and a path destination. Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
|
Reply to the review finding outside the diff (a non-blocking source with a path destination).
|
There was a problem hiding this comment.
I re-reviewed after 79a1f99 and found no new bugs; the second commit's move of the wait into shared read_retrying/write_retrying (dropping wait_for_kernel_copy) and the {"exit":$?} stage in the test pipeline address two of my earlier inline threads. A human look is still worthwhile because the remaining open threads are design trade-offs (an unbounded wait on a pool thread where the base rejected at once), and the macOS select$DARWIN_EXTSN$NOCANCEL path first runs on CI.
What was reviewed:
- The new EAGAIN arm in
do_copy_file_range:remainis already decremented for a known size andtotal_writtenis passed by reference, so the fallback loop continues from the byte sendfile/splice stopped at;CopyFileRangeis excluded, so two regular files never wait. read_retrying/write_retrying: on a failed wait they return the original EAGAIN, andis_retry()is a plain errno == EAGAIN check, so no other errno is retried.- The macOS
wait_until: EINTR is retried, the inline set isFD_SETSIZE/32words with a heapVecabove that, and the*mut u32extern matches the DARWIN_EXTSN bit-array ABI — but this is#[cfg(target_os = "macos")]and could not be compiled or run here. write_all_to_stdiorefactor is behavior-preserving (samepoll(-1)on POLLOUT via the new helper).
Extended reasoning...
The PR touches the bun_sys syscall layer (new poll/select wait helpers and a new select$DARWIN_EXTSN$NOCANCEL extern on macOS), the Linux sendfile/splice copy loop in copy_file.rs, the node:fs read/write copy loop, and adds a large describe.concurrent block of shell-pipeline subprocess tests; it touches no auth, crypto, or injection surface. Since my prior review the second commit replaced the per-syscall poll-and-retry with a fallback to the shared read/write loop, which removes the POLLERR retry logic I flagged, and the tests now assert the Bun child's own exit status. No new findings surfaced this run (exit reason dry_streak) and none of the changed files are under CODEOWNERS. Deferring rather than approving because the change converts a fast EAGAIN rejection into an unbounded park of a work-pool thread (an acknowledged, still-open design trade-off), and the platform-gated macOS select path cannot be type-checked or exercised from this Linux checkout.
Still open from earlier reviews (2):
- Unresolved: 1 minor or pre-existing, 1 blocking on lines changed since (possibly already fixed).
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
|
The re-review of 79a1f99 has no new findings. Two earlier threads were still open. Both are design trade-offs that this PR does not change in code, so I resolved them. The Downsides section has both, with numbers.
The replies in the two threads have the measurements. The notes in the description have a question for the maintainer about the pool thread. |
Problem
await Bun.write(Bun.stdout, Bun.file(path))rejects withEAGAIN(syscallsendfile) after 65536 bytes when fd 1 is a non-blocking pipe. Any use ofprocess.stdoutsets that flag (src/io/openForWriting.rs:157).CopyFile, the work-pool job behindBun.write(file, file), has no case for EAGAIN:do_copy_file_range(src/runtime/webcore/blob/copy_file.rs:477), the read/write loops (:990,node_fs.rs:4883).Fix
bun_sys::read_retryingandwrite_retryingwait on EAGAIN, then repeat the call.read_write_loop_cappedand the node:fs loop call them.sendfileorsplicethe copy continues in that loop, as in dylan-conway's 90e0621 (console: output goes behind what process.stdout/stderr have buffered, as in Node #37128).test/js/bun/io/bun-write.test.js(7 new tests, all fail on main). Alsoprocess-stdio.test.ts,fs/cp.test.ts, shellcp.test.ts, source lints,rust:check-all.pollcannot wait for.Background
poll. macOS usesselect, becausepollthere does not wake when the other end of a named pipe closes.sendfileandspliceafter a wait. Review found two busy loops in that wait.Downsides
worker.terminate()then takes 1.68 s (main: 0 ms, the copy failed).process.stdoutstill holds: 14 of 20 runs (main: 6 of 20, and all 20 reject).sendfile. Without EAGAIN the syscalls are the same.Notes
Repro (Linux, bun 1.4.3-canary.1+367d939d9 and main 179664e, 3 of 3 runs each)
main prints
rejected EAGAIN (sendfile)and65536. This PR printsresolved 1048576and1048576.Reach. No issue reports this: searches for
EAGAIN sendfile,EAGAIN spliceandEAGAIN "Bun.write"find 0 issues. It was found during work on #41685. It is deterministic for a file above 65536 bytes and a pipe.Who sets the flag, and who clears it
process.stdoutorprocess.stderrin the script or in a dependency, for example a read ofisTTY.( bun -e "process.stdout.isTTY"; bun cat.mjs ) | wc -cfails in the second process on main.socat EXEC:start a server. The use ofprocess.stdoutmakes stdin non-blocking too.Bun.spawnmakes its own end of an extra"pipe"non-blocking (proc.stdio[3]).src/spawn_sys/spawn_process.rs:793, since process.stdout/stderr: clear O_NONBLOCK for spawned children, don't drop console output on EAGAIN #43868). A copy after a spawn with an inherited stdout resolves on main.How the copy waits
read_retryingandwrite_retrying(src/sys/lib.rs) make onereadorwrite. On EAGAIN,wait_until_readableorwait_until_writableblocks the thread with no timeout, and the call repeats. If the wait fails, the helper returns the EAGAIN.poll. On macOS it isselect$DARWIN_EXTSN$NOCANCEL, which has no FD_SETSIZE limit.write_all_to_stdio(process.stdout/stderr: clear O_NONBLOCK for spawned children, don't drop console output on EAGAIN #43868) calls the same wait in place of its ownpoll.read_write_loop_cappedcopies into an fd destination.NodeFS::copy_file_using_read_write_loopcopies into a path destination. Both call the helpers for each read and write.do_copy_file_range: EAGAIN fromsendfileorsplicereturnsfallback_read_write(remain, unknown_size, &mut total_written), as ENOSYS and EXDEV do. The loop continues at the byte where the kernel copy stopped.copy_file_rangehas no such arm: it runs between two regular files.fs.copyFile,fs.cpand Bun Shellcp. Its 5 other call sites open both ends from paths after an ISREG check of the source. On Linux a path gives a new blocking description, so those calls cannot get EAGAIN. On macOS and FreeBSD a destination such as/dev/stdoutshares the description of fd 1, so there the loop now waits where it failed with EAGAIN. That was not run.Tests. Each test spawns a child that reports
{ ready }and then how the promise settled. A child in a pipeline also reports its exit status.sendfilecopies 65536 bytes, fails with EAGAIN, and the loop continues at that byte. The child measures its CPU time for 1 s while the copy waits.splice.splicefails with EAGAIN at byte 0.EPIPE write.process.stdout.Bun.write(path, Bun.stdin)gets three pieces, each after the one before it is in the file, and resolves with all 12 bytes. The file at the path has 5 bytes and a known size. The second piece ends in the sized part of the node:fs loop, the third in the part after it.On main all 7 fail: 1 of 1 run of a debug build and 15 of 15 runs of a release build. On this PR all 7 pass in 30 of 30 runs of a release build and in 4 of 4 runs of a debug build. They also pass in 2 of 2 runs with the environment of the ASAN lane.
Each part of the fix breaks a test when it is removed (debug builds):
do_copy_file_range: tests 1 to 6 fail withEAGAIN sendfileorEAGAIN splice.writeforwrite_retryinginread_write_loop_capped: tests 1, 2, 4 and 6 fail withEAGAIN write.readforread_retryinginread_write_loop_capped: test 5 fails withEAGAIN read.readforread_retryingin the node:fs loop: test 7 fails afterhel(the sized part) or afterhello(the part after it).writesites of the node:fs loop have no test. They can get EAGAIN only on macOS and FreeBSD, through a path that shares the description of fd 1.Measurements (release builds, linux x64, main 179664e against this PR)
pollandselectcalls: 0.bench/cat/bun.jsto a file and to a pipe, and stdin to stdout. The same three withBUN_CONFIG_DISABLE_COPY_FILE_RANGE=1.bench/copyfile/bun.jswith and without that variable. 200 calls offs.copyFileSync. A file toBun.file(3).sendfile,splice,copy_file_range,read,poll,ppoll,select,pselect6,fcntl,ftruncate,fstat,openat. Thewritecount changes by one between runs on both builds.stepi.read_write_loop_capped: 47 to 77, with 11 to 13 conditional branches. The node:fs loop: 61 to 69, with 11 to 12. Most of the difference is awritethat is now a call.do_copy_file_rangewithsendfile: 33 instructions and 5 conditional branches for each call, on both builds. The twocopy_file_rangeinstances have the same size. Thesendfileandspliceinstances grow by 2 bytes or 0.size: text 107,557,538 to 107,558,818 bytes (+1,280).struct CopyFile, itsJobContextimpl andCopyFile::createhave no hunk: no new allocation, queue entry or field.| cat > /dev/nullafter a use ofprocess.stdout. Medians of 12 interleaved runs, with the range: this PR 370 ms (224 to 709). The first revision, which stays onsendfile: 266 ms (159 to 536). main with a blocking fd 1: 192 ms (148 to 414).sendfilecall fails with EAGAIN. Then 64readand 64writecalls copy. 42writecalls fail with EAGAIN, and there are 42pollcalls.splicecall, then areadthat blocks. 10 to 16 ms of CPU. The copy resolves with 0 when stdin ends.worker.terminate()while a copy in the worker waits for a reader that starts after 2 s. main with a blocking fd 1: it settles after 1.65 s. main with a non-blocking fd 1: after 0 ms, because the copy rejected. This PR with a non-blocking fd 1: after 1.68 s.process.exit()on the main thread does not wait: the process is gone after 336 ms (main with a blocking fd 1: 337 ms).fs.promises.statbehind 12 copies that wait (12 pool threads): it settles when the reader starts, on this PR and on main with a blocking fd 1.process.stdout.write("A".repeat(200000)), then a copy of a 100 KiB file ofB:Bbytes are inside the A run in 6.B.process.stdout.writefirst, 20 runs: in order 20 of 20. main rejects 20 of 20.objdump,size,nm. The machine has nostraceorperf.Not run here. No macOS or FreeBSD machine was available.
cargo checkpasses for all 12 targets. On macOS, tests 1, 2, 4, 5 and 6 run theselectwait, and tests 1 and 5 put the CPU limit on it. Windows is not affected and not run: only POSIX code sets O_NONBLOCK on stdio, andCopyFileWindowsis other code.An idle source whose reader left. The first revision rejected with EPIPE at once, because
sendfilelooks at the pipe first. Now the copy is inreadand waits for input, ascatdoes. EPIPE comes with the next byte, or the copy resolves when the source ends.Considered. A park on the io thread, as
WriteFiledoes: every copy pays a job-list entry, and the loops still need a wait for the fds that cannot park. A wait that keepssendfileandsplice(the first revision): it saves the CPU time above and needs a rule for the end to wait for.Other paths of
Bun.writewith the same trigger. This PR changes only the file-to-file job.Bun.write(Bun.stdout, ...)duplicates the first 8192 bytes after a partial pipe write #45663, Bun.write: after EAGAIN continue with the unwritten tail, the count and the open fd #44389).WriteFile::do_writerepeatswritewith no wait when it does not know that the fd can block (Bun.write: poll instead of spin when a nonblocking stdout pipe returns EAGAIN #35953).wait_for_stdin). The one of the two PRs that lands second can callbun_sys::wait_until_readable(Fd::stdin())in its place. io: wait for a named pipe's EOF with select(2) on macOS #40099 declares the sameselect$DARWIN_EXTSN$NOCANCEL, which is a one-line conflict for the second.Bun.write(Bun.stdout, ...)has its own writer and does not go behind the data thatprocess.stdoutholds. console: output goes behind what process.stdout/stderr have buffered, as in Node #37128 keeps that: its list says thatBun.write(Bun.stdout)has its own sink, as on main.Still wrong after this PR
( bun -e "process.stdout.isTTY"; cat big.bin ) | wc -cprintscat: write errorafter 65536 bytes.pollreturns at once for it, so the copy repeatswriteon a full core until the peer reads: 1493 ms of CPU in 1500 ms. Then it completes. main rejects with EAGAIN.console.logon main has the same loop since process.stdout/stderr: clear O_NONBLOCK for spawned children, don't drop console output on EAGAIN #43868 (1472 ms in 1479 ms). Another process must set SO_ZEROCOPY or SO_TIMESTAMPING on the socket for this: Bun has no API for them. A guard needs a count of retries in the helpers, and no test on a lane can reach it.fcopyfilerefuses it with ENOTSUP before a loop runs (Writingstdinto a file errors withENOTSUP#14054)./dev/stdoutwith a regular source goes throughfcopyfile. Its loop in Apple'scopyfile.cends at awritethat fails.Non-regular files aren't supported yet(Bun.write(file, Bun.stdin): splice a pipe source into any destination on Linux #36692).sendfile,spliceandcopy_file_rangestill ends the copy (node:fs, Bun.write, fetch: retry EINTR in the bun_sys wrappers instead of surfacing it #45929).History. #38498 (closed in favour of #37128) had the wait and most of these tests. #43868 made the console writer wait in
pollon EAGAIN (write_all_to_stdio). #37128 had this fix in commit 90e0621: EAGAIN fromsendfileandsplicewent to the read/write loop, andread_retryingandwrite_retryingwaited in both loops. Commit 9033ecd cut that PR down to the console change. This PR has that shape again. It adds theselectwait for macOS and leaves out the EINTR arm, which #45929 owns.Bun.write(Bun.stdout, Bun.file(path).stream())is a workaround on main: it goes through the stream sink and the event loop.Questions for @dylan-conway
NodeFS::copy_file_using_read_write_loopstill the place that you want? Ifnode_fs.rsmust stay as it is, the wait can move toCopyFile. Its calls withstat_size0 can useread_write_loop_capped.sendfile. Remove libuv on Windows #42819 addsJobContext::waitsandWorkPool::schedule_waitfor a job that waits for the other end of a pipe. This copy can reportwaitsfor a pipe or socket when that lands. Run pool jobs that own their memory without a VM borrow so worker.terminate() does not wait on a blocked Bun.write(file, file) #38312 is aboutworker.terminate()and a copy that blocks.Overlap. #41685, #46636 and #45929 edit other lines of
do_copy_file_range.copy_file.rsmerges with each of them without a conflict (git merge-file). The 3-argumentfallback_read_writethat #41685 and #46636 call is the same.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/io/bun-write.test.js